Listen for ctrl-c during provider request#5585
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors interrupt handling to centralize Ctrl+C signal handling through a cancellation token pattern, and adds support for EOF (End of File) signal handling in user input.
Key changes:
- Centralized Ctrl+C handling via a spawned task that cancels a token instead of directly handling the signal in the select loop
- Modified the event loop to listen for cancellation token instead of raw Ctrl+C signals
- Added EOF signal handling to exit the session gracefully
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/goose-cli/src/session/mod.rs | Refactored interrupt handling to use a spawned task for Ctrl+C that triggers cancellation token, and updated the select loop to listen for cancellation instead of raw signals |
| crates/goose-cli/src/session/input.rs | Added EOF (Ctrl+D) signal handling to exit the session, matching the behavior of Ctrl+C when the line is empty |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let cancel_token_interrupt = cancel_token.clone(); | ||
| tokio::spawn(async move { | ||
| if tokio::signal::ctrl_c().await.is_ok() { | ||
| cancel_token_interrupt.cancel(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The spawned task is not tracked or joined, which could lead to it outliving the function scope. If process_agent_response returns before Ctrl+C is pressed, the spawned task will continue running and may trigger cancellation on an already-consumed token. Consider storing the task handle and aborting it before the function returns, or using tokio::select! with a scoped spawn pattern to ensure proper cleanup.
crates/goose-cli/src/session/mod.rs
Outdated
| tokio::spawn(async move { | ||
| if tokio::signal::ctrl_c().await.is_ok() { | ||
| cancel_token_interrupt.cancel(); |
There was a problem hiding this comment.
The spawned task silently ignores any errors from the spawn itself and only checks if ctrl_c succeeded. Consider handling potential spawn failures or at minimum documenting why they can be safely ignored. Also, the task only handles a single Ctrl+C signal - subsequent signals won't be caught. Consider using a loop to handle multiple interrupts or documenting this single-signal behavior.
| tokio::spawn(async move { | |
| if tokio::signal::ctrl_c().await.is_ok() { | |
| cancel_token_interrupt.cancel(); | |
| // Spawn a background task to listen for Ctrl+C signals and cancel the token. | |
| // We loop to handle multiple interrupts, and log any errors from ctrl_c. | |
| // Note: tokio::spawn is infallible unless the runtime is shutting down, in which case | |
| // the process is likely exiting anyway, so we can safely ignore errors from spawn. | |
| tokio::spawn(async move { | |
| loop { | |
| match tokio::signal::ctrl_c().await { | |
| Ok(_) => { | |
| cancel_token_interrupt.cancel(); | |
| } | |
| Err(e) => { | |
| // Log the error and break the loop. | |
| eprintln!("Failed to listen for Ctrl+C: {}", e); | |
| break; | |
| } | |
| } |
* main: (41 commits) Add pending extension indicator to extension panel (#5493) Add environment subsition for auth blocks (#5439) acp: ToolCallLocations and working cancellation (#5588) feat(providers): add Mistral AI provider (#5009) Listen for ctrl-c during provider request (#5585) Also accept null as description, not just missing (#5589) Document missing recipe param types (#5584) docs: description required for "Add Extension" in cli (#5573) fix: Add schema-aware numeric coercion for MCP tool arguments (#5478) Add uv for uvx in Justfile (#5581) Keep llm logs in place (#5577) bump to 1.12.0 (#5580) automate more of the release process (#5409) add clippy warning for string_slice (#5422) improve linux tray icon support (#5425) feat: log rotation (#5561) use app.isPackaged instead of checking for node env development (#5465) disable RPM build-ID generation to prevent package conflicts (#5563) Add Diagnostics Info to Q&A and Bug Report Templates (#5565) fix: improve server error messages to include HTTP status code (#5532) ...
* main: (53 commits) acp: ToolCallLocations and working cancellation (#5588) feat(providers): add Mistral AI provider (#5009) Listen for ctrl-c during provider request (#5585) Also accept null as description, not just missing (#5589) Document missing recipe param types (#5584) docs: description required for "Add Extension" in cli (#5573) fix: Add schema-aware numeric coercion for MCP tool arguments (#5478) Add uv for uvx in Justfile (#5581) Keep llm logs in place (#5577) bump to 1.12.0 (#5580) automate more of the release process (#5409) add clippy warning for string_slice (#5422) improve linux tray icon support (#5425) feat: log rotation (#5561) use app.isPackaged instead of checking for node env development (#5465) disable RPM build-ID generation to prevent package conflicts (#5563) Add Diagnostics Info to Q&A and Bug Report Templates (#5565) fix: improve server error messages to include HTTP status code (#5532) improvement: add useful error message when attempting to use unauthenticated cursor-agent (#5300) fix: unblock acp via databricks (#5562) ...
* main: fix: customised recipe to yaml string to avoid minininjia parsing error (#5494) Add pending extension indicator to extension panel (#5493) Add environment subsition for auth blocks (#5439) acp: ToolCallLocations and working cancellation (#5588) feat(providers): add Mistral AI provider (#5009) Listen for ctrl-c during provider request (#5585) Also accept null as description, not just missing (#5589) Document missing recipe param types (#5584) docs: description required for "Add Extension" in cli (#5573) fix: Add schema-aware numeric coercion for MCP tool arguments (#5478) Add uv for uvx in Justfile (#5581) Keep llm logs in place (#5577) bump to 1.12.0 (#5580) automate more of the release process (#5409)
* origin/main: (75 commits) fix: customised recipe to yaml string to avoid minininjia parsing error (#5494) Add pending extension indicator to extension panel (#5493) Add environment subsition for auth blocks (#5439) acp: ToolCallLocations and working cancellation (#5588) feat(providers): add Mistral AI provider (#5009) Listen for ctrl-c during provider request (#5585) Also accept null as description, not just missing (#5589) Document missing recipe param types (#5584) docs: description required for "Add Extension" in cli (#5573) fix: Add schema-aware numeric coercion for MCP tool arguments (#5478) Add uv for uvx in Justfile (#5581) Keep llm logs in place (#5577) bump to 1.12.0 (#5580) automate more of the release process (#5409) add clippy warning for string_slice (#5422) improve linux tray icon support (#5425) feat: log rotation (#5561) use app.isPackaged instead of checking for node env development (#5465) disable RPM build-ID generation to prevent package conflicts (#5563) Add Diagnostics Info to Q&A and Bug Report Templates (#5565) ...
* main: (60 commits) fix: add standard context menu items to prevent empty right-click menu (#5616) Bump openapi in prepare-release (#5611) docs: add access control section to Developer tutorial (#5615) Token state not showing on load, or after message is finished. (#5606) Change the other location too (#5608) feat(ui): bring back quick launcher (#5144) Support platform tools through CLI (#5570) Avoid web double write (#5601) fix: gemini flash -> pro for mcp smoke tests (#5574) Manual compaction test and fix (#5568) fix: tidy up claude cli handling (#5594) Remove jetbrains (#5602) feat(githubcopilot): add support for newer Copilot AI Models (#5603) fix: customised recipe to yaml string to avoid minininjia parsing error (#5494) Add pending extension indicator to extension panel (#5493) Add environment subsition for auth blocks (#5439) acp: ToolCallLocations and working cancellation (#5588) feat(providers): add Mistral AI provider (#5009) Listen for ctrl-c during provider request (#5585) Also accept null as description, not just missing (#5589) ...
Signed-off-by: fbalicchia <fbalicchia@gmail.com>
Signed-off-by: Blair Allan <Blairallan@icloud.com>
Prior, we'd only start listening during the stream. So if the user hit ctrl-c while we waited for the provider response (before any streaming started), we wouldn't handle it.
fixes #4265
Also treats ctrl-D as exit, so we don't print "Error: EOF" when you exit this way.